Add datetime dialog examples for date/datetime field features#205
Add datetime dialog examples for date/datetime field features#205
Conversation
Implements 3 datetime dialog groups matching webhook test server structure: - /dialog datetime-basic: Basic date/datetime, min date, intervals, relative dates - /dialog datetime-ranges: Date/datetime ranges (horizontal, vertical, single-day) - /dialog datetime-timezone: Timezone support and manual time entry Covers test cases MM-T2530A through MM-T2530S (excluding exclusions) Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
Remove the datetime-ranges dialog group (range selection with horizontal/vertical layouts) which is not yet supported. Simplify timezone dialog comments and introduction text. Upgrade server/public from pre-release to v0.3.0, removing the local replace directive, and bump Go to 1.25.8 with updated dependencies. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughUpdated Go toolchain and dependency versions in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant MattermostServer
participant Plugin
User->>Client: Issue "/dialog datetime-basic" or "/dialog datetime-timezone"
Client->>MattermostServer: Send command
MattermostServer->>Plugin: Deliver command callback
Plugin->>Plugin: build dialog via getDialogDateTime*( )
Plugin->>MattermostServer: Open dialog (POST to /dialog/3)
MattermostServer->>Client: Render dialog UI
User->>Client: Interact and submit dialog
Client->>MattermostServer: Submit dialog payload
MattermostServer->>Plugin: Forward dialog submission callback
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/dialog_samples.go (1)
1022-1089: Align the MM-T2530 labels with the actual samples.The coverage note says this block maps to
MM-T2530A/B/C/D/F/G/H, but the inline labels only markA/B/C/D/Fand tag both relative examples asMM-T2530F. That will make it harder to correlate e2e failures back to the right dialog sample.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/dialog_samples.go` around lines 1022 - 1089, The comment notes misaligned MM-T2530 labels in getDialogDateTimeBasic: update the inline comment tags for each DialogElement so they correctly map to MM-T2530A, B, C, D, F, G, H (rather than duplicating F). Specifically, change the duplicate "MM-T2530F" tags on the two relative examples so one is MM-T2530F (relative date) and the other is MM-T2530G or H as appropriate (relative_datetime -> assign the correct remaining label), and ensure the top-of-function coverage list still matches the element tags; adjust the DisplayName/Name comments if needed to make correlation to test cases unambiguous.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/dialog_samples.go`:
- Around line 1124-1142: Update the HelpText for the dialog items with Name
"london_dropdown" and "london_manual" (DisplayName "London Office Hours
(Dropdown)" and "London Office Hours (Manual Entry)") so they don’t say “GMT”;
instead reference UK local time or Europe/London time (e.g., "Times shown in UK
local time" and "Type time in UK local time") to accurately reflect
model.DialogDateTimeConfig.LocationTimezone = "Europe/London" and timezone DST
changes. Ensure both HelpText strings are updated consistently.
---
Nitpick comments:
In `@server/dialog_samples.go`:
- Around line 1022-1089: The comment notes misaligned MM-T2530 labels in
getDialogDateTimeBasic: update the inline comment tags for each DialogElement so
they correctly map to MM-T2530A, B, C, D, F, G, H (rather than duplicating F).
Specifically, change the duplicate "MM-T2530F" tags on the two relative examples
so one is MM-T2530F (relative date) and the other is MM-T2530G or H as
appropriate (relative_datetime -> assign the correct remaining label), and
ensure the top-of-function coverage list still matches the element tags; adjust
the DisplayName/Name comments if needed to make correlation to test cases
unambiguous.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 78ea98e0-31ae-455e-a74a-e28684904b19
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
go.modserver/command_hooks.goserver/dialog_samples.go
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/dialog_samples.go (1)
1030-1030: Prefer HTTPS for new icon URLs.Line 1030 and Line 1111 currently use
http://. Switching tohttps://avoids mixed-content/security warnings in stricter environments.Suggested patch
- IconURL: "http://www.mattermost.org/wp-content/uploads/2016/04/icon.png", + IconURL: "https://www.mattermost.org/wp-content/uploads/2016/04/icon.png", ... - IconURL: "http://www.mattermost.org/wp-content/uploads/2016/04/icon.png", + IconURL: "https://www.mattermost.org/wp-content/uploads/2016/04/icon.png",Also applies to: 1111-1111
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/dialog_samples.go` at line 1030, Update the IconURL fields that currently use "http://www.mattermost.org/..." to use "https://www.mattermost.org/..." to avoid mixed-content/security warnings; locate the IconURL assignments in the dialog sample structs (the occurrences around the IconURL field at the places edited in dialog_samples.go, including the instance near IconURL: "http://www.mattermost.org/wp-content/uploads/2016/04/icon.png" and the other occurrence around line 1111) and change the scheme to https for each.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/dialog_samples.go`:
- Line 1030: Update the IconURL fields that currently use
"http://www.mattermost.org/..." to use "https://www.mattermost.org/..." to avoid
mixed-content/security warnings; locate the IconURL assignments in the dialog
sample structs (the occurrences around the IconURL field at the places edited in
dialog_samples.go, including the instance near IconURL:
"http://www.mattermost.org/wp-content/uploads/2016/04/icon.png" and the other
occurrence around line 1111) and change the scheme to https for each.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dbe6d409-0126-4d8b-b28f-3cbcbb83b29f
📒 Files selected for processing (1)
server/dialog_samples.go
| Type: "date", | ||
| HelpText: "Must be today or later", | ||
| Placeholder: "Select a future date", | ||
| MinDate: "today", |
| HelpText: "Times shown in Europe/London time - select from 60 min intervals", | ||
| DateTimeConfig: &model.DialogDateTimeConfig{ | ||
| LocationTimezone: "Europe/London", | ||
| TimeInterval: 60, |
There was a problem hiding this comment.
I also see a TimeInterval in DialogElement. What is the difference?
There was a problem hiding this comment.
Plan to deprecate TimeInterval in DialogElement along with minDate and maxDate, so all DateTime settings are in DateTimeConfig. TimeInterval in DateTimeConfig takes presidance, but if absent, the one in DialogElement is used.
| Type: "datetime", | ||
| HelpText: "Type any time: 9am, 14:30, 3:45pm - no rounding", | ||
| DateTimeConfig: &model.DialogDateTimeConfig{ | ||
| AllowManualTimeEntry: true, |
There was a problem hiding this comment.
Nit: It's not clear by the name what this does. Is there a name that makes the indent more clear?
There was a problem hiding this comment.
I'm open to suggestions, just "ManualTimeEntry"? Its already included in WebApp, but I can change it and deprecate this one.
Summary